Feat/stateful goroutines exercise #112
Conversation
This commit implements issue zhravan#71: Add exercise templates and tests for Stateful Goroutines concept. Changes: - Added exercise template at internal/exercises/templates/28_stateful_goroutines/ - stateful_goroutines.go: Incomplete template with TODO comments demonstrating the stateful goroutines pattern using channels for state management - stateful_goroutines_test.go: Comprehensive test suite covering: * Counter initialization * Basic increment operations * Concurrent increments from multiple goroutines * Concurrent reads and writes * Negative increment support - Added working solution at internal/exercises/solutions/28_stateful_goroutines/ - Implements the channel-based state management pattern - Uses a single goroutine to own state (avoiding race conditions) - readOp and writeOp structs for communication via channels - select statement for handling concurrent operations - Updated internal/exercises/catalog.yaml - Added slug: 28_stateful_goroutines - Title: Stateful Goroutines - Comprehensive hints explaining the pattern: * Channel-based communication * readOp and writeOp structs with response channels * State-owning goroutine with select * Avoiding mutexes through single-goroutine ownership Testing: - Template tests fail as expected (4/5 tests fail due to incomplete implementation) - Solution passes all tests (5/5 tests pass) - Tests validate concurrent safety and correctness This exercise teaches the Go concurrency pattern of using goroutines and channels to manage shared state, aligning with Go's principle of 'share memory by communicating' rather than using explicit locks.
- Fix whitespace formatting in solution file (remove trailing spaces) - Update .gitignore to exclude Codacy artifacts: - .codacy/ directory - .github/instructions/ directory These files are tool-generated and should not be committed to the repository.
WalkthroughAdds a new "Stateful Goroutines" exercise (47): catalog entry with hints, exercise template and tests, a complete reference solution implementing a channel-driven Counter, and small .gitignore updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Run 'go fmt ./...' to fix formatting issues in test file. This resolves the CI/CD pipeline failure caused by improper code formatting. Fixed file: - internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (corrected whitespace formatting)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
62-101: Consider removing time.Sleep calls for faster, equally effective tests.The
time.Sleep(time.Microsecond)calls at lines 76 and 88 attempt to interleave operations but don't provide stronger guarantees than the Go scheduler already provides. Removing them would make tests faster without reducing coverage.Apply this diff to remove the sleeps:
defer wg.Done() for j := 0; j < 5; j++ { counter.Increment(1) - time.Sleep(time.Microsecond) } }() } // Start readers for i := 0; i < numReaders; i++ { wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { _ = counter.GetValue() - time.Sleep(time.Microsecond) } }() }If you prefer to keep them, the test remains correct.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
19-60: Consider timeout handling for production use.While the current implementation correctly demonstrates the stateful goroutine pattern, production code should consider adding timeout mechanisms to prevent indefinite blocking if the state goroutine becomes unresponsive.
Example approach using context:
func (c *Counter) GetValueWithContext(ctx context.Context) (int, error) { read := readOp{resp: make(chan int)} select { case c.reads <- read: select { case value := <-read.resp: return value, nil case <-ctx.Done(): return 0, ctx.Err() } case <-ctx.Done(): return 0, ctx.Err() } }This is optional for the teaching exercise but valuable for real-world use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)internal/exercises/catalog.yaml(1 hunks)internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (2)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(20-41)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(26-29)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (2)
NewCounter(26-29)Counter(20-23)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (1)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (2)
NewCounter(20-41)Counter(14-17)
🔇 Additional comments (12)
.gitignore (1)
33-37: Ignore rules look good.Switching to
bin/properly catches build artifacts, and adding the Codacy/GitHub instruction directories keeps generated/config outputs out of version control. ✔️internal/exercises/catalog.yaml (1)
137-144: LGTM! Clear and well-structured catalog entry.The hints provide a logical progression explaining the channel-based state management pattern, from operation channels through to the rationale for avoiding mutexes.
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (3)
24-34: LGTM! Clean sequential increment test.The test correctly validates basic increment functionality without introducing artificial delays or complexity.
36-60: LGTM! Thorough concurrent increment test.The test validates the core concurrency safety guarantee of the pattern with 100 goroutines performing 1,000 total increments. The use of
sync.WaitGroupensures proper synchronization.
103-113: LGTM! Validates negative increment handling.The test correctly verifies that negative amounts work as expected (effectively decrementing the counter).
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (4)
1-8: LGTM! Clear introduction and requirements.The TODO comment effectively sets expectations and explains the core concept of single-goroutine state ownership with channel-based communication.
9-23: LGTM! Type definitions provide necessary structure.The complete type definitions (readOp, writeOp, Counter) give learners the concrete scaffolding they need to implement the channel-based state management pattern, while leaving the core logic as exercises.
25-29: Template stub is intentionally incomplete (as expected).Returning an uninitialized
Counter{}with nil channels will cause panics when tests callIncrementorGetValue, which is expected behavior for an incomplete template—learners must initialize the channels and start the goroutine.
31-40: LGTM! Clear method stubs with helpful TODOs.The TODO comments guide learners to implement the channel send/receive pattern for writes (with confirmation) and reads (with value return). The stubs are appropriately incomplete for a teaching template.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (3)
3-17: Type definitions match template correctly.The types align with the template scaffolding, ensuring consistency between the learning exercise and the reference solution.
43-51: LGTM! Correct synchronous write implementation.The method correctly creates a one-time response channel, sends the write operation, and blocks until acknowledged. This ensures the increment completes before returning.
53-60: LGTM! Correct synchronous read implementation.The method correctly creates a one-time response channel, sends the read operation, and blocks until the state value is returned. The pattern mirrors
Incrementappropriately.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go
Show resolved
Hide resolved
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go
Outdated
Show resolved
Hide resolved
- Remove unnecessary time.Sleep from TestCounterInitialization test - Add Close() method to Counter to prevent goroutine leaks - Add done channel to properly terminate state-owning goroutine Fixes review comments from zhravan#112 (review)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
59-98: Consider removing the time.Sleep calls for faster, more robust tests.The
time.Sleep(time.Microsecond)calls at lines 73 and 85 artificially slow down concurrent operations. The channel-based implementation should naturally handle rapid concurrent reads and writes without artificial delays. Removing these sleeps will make the test faster and eliminate potential timing-dependent behavior.Apply this diff to remove the sleeps:
wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { counter.Increment(1) - time.Sleep(time.Microsecond) } }() } // Start readers for i := 0; i < numReaders; i++ { wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { _ = counter.GetValue() - time.Sleep(time.Microsecond) } }() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(21-45)
🔇 Additional comments (4)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (4)
9-19: LGTM!The initialization test correctly validates that
NewCounter()returns a non-nil counter and that the initial value is 0. The test is straightforward and correct.
21-31: LGTM!The test correctly validates basic increment functionality in a single-threaded context. The logic is sound: two increments (5 and 3) should result in a total of 8.
33-57: LGTM!The concurrent increments test properly validates thread-safety with 100 goroutines performing 10 increments each. The use of
WaitGroupensures all operations complete before the final assertion, and the expected value calculation (1000) is correct.
100-110: LGTM!The test correctly validates that
Incrementhandles negative deltas. The logic is sound: incrementing by 10 then by -3 should result in a final value of 7.
|
@aviralgarg05: Can you change and resolve current conflicts? |
…47) and resolve merge conflicts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/exercises/solutions/47_stateful_goroutines/stateful_goroutines.go`:
- Around line 14-68: The Close method must be made idempotent and
Increment/GetValue must avoid blocking after Close: add a sync.Once field (e.g.,
c.once) and a completion signal channel (e.g., c.closed) to the Counter struct,
use c.once.Do() in Close to close c.done and then wait/close c.closed after the
state goroutine exits, and in Increment and GetValue check whether done/closed
is already closed (non-blocking select on c.done or c.closed) before attempting
to send on c.writes/c.reads to return early (or return zero/failure) to prevent
sends to unreceived channels; update NewCounter to initialize once and closed
and ensure the state goroutine closes/signals c.closed when it returns.
🧹 Nitpick comments (1)
internal/exercises/templates/47_stateful_goroutines/stateful_goroutines.go (1)
20-39: Keep template/solution API parity.The solution adds
Close, but the template doesn’t expose it. Consider adding aCloseTODO to the template (or droppingClosefrom the solution) so learners see a consistent public API. Based on learnings, templates and solutions should align for learners.
internal/exercises/solutions/47_stateful_goroutines/stateful_goroutines.go
Show resolved
Hide resolved
2 participants
Summary
Feat/stateful goroutines exercise
Changes:
Added exercise template at internal/exercises/templates/28_stateful_goroutines/
the stateful goroutines pattern using channels for state management
Added working solution at internal/exercises/solutions/28_stateful_goroutines/
Updated internal/exercises/catalog.yaml
Testing:
This exercise teaches the Go concurrency pattern of using goroutines and channels
to manage shared state, aligning with Go's principle of 'share memory by communicating'
rather than using explicit locks.
Checklist
make verifyorgolearn verify <slug>Screenshots / Output (if CLI UX)
Paste before/after where helpful.
Related issues
Fixes #71
For Hacktoberfest 2025
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.